Skip to content

the unadjusted ABI needs to pass aggregates by-value #118127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 25, 2023
Merged

the unadjusted ABI needs to pass aggregates by-value #118127

merged 2 commits into from
Nov 25, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 21, 2023 •
edited
Loading

Fixes #118124, a regression introduced in #117500

@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2023

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 21, 2023
@RalfJung RalfJung mentioned this pull request Nov 21, 2023
Closed
@Noratrieb
Copy link
Member

Could you just add a test that uses the NEON intrinsic and ensures it doesn't fail to compile?

@RalfJung
Copy link
Member Author

I can probably do a no_core test that works on all hosts...

@bjorn3
Copy link
Member

bjorn3 commented Nov 21, 2023

Is the unadjusted ABI marked as internal feature? If not I think we should mark it as such.

@RalfJung
Copy link
Member Author

Not currently:

rust/compiler/rustc_feature/src/unstable.rs

Lines 162 to 163 in efe54e2

/// Allows using the `unadjusted` ABI; perma-unstable.
(unstable, abi_unadjusted, "1.16.0", None, None),

But I agree.

@RalfJung RalfJung mentioned this pull request Nov 22, 2023
Closed
@RalfJung RalfJung mentioned this pull request Nov 22, 2023
Merged
@RalfJung
Copy link
Member Author

Could you just add a test that uses the NEON intrinsic and ensures it doesn't fail to compile?

I added a test.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 23, 2023
@matthiaskrgr
Rollup merge of rust-lang#118167 - RalfJung:unadjusted-abi-is-interna…
9e944c8 
…l, r=petrochenkov

make the 'abi_unadjusted' feature internal

As [suggested](rust-lang#118127 (comment)) by `@bjorn3.`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2023
@rust-timer
Unrolled build for rust-lang#118167
46522a1 
Rollup merge of rust-lang#118167 - RalfJung:unadjusted-abi-is-internal, r=petrochenkov

make the 'abi_unadjusted' feature internal

As [suggested](rust-lang#118127 (comment)) by `@bjorn3.`
@RalfJung
Copy link
Member Author

@davidtwco or anyone else from @rust-lang/compiler , this fixes a critical codegen regression, would be good to land it ASAP. :)

@ytmimi ytmimi mentioned this pull request Nov 25, 2023
Closed
@compiler-errors
Copy link
Member

@bors r+ rollup=never p=1

easier to bisect that way

@bors
Copy link
Collaborator

bors commented Nov 25, 2023

📌 Commit ebfb95a has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2023
@bors
Copy link
Collaborator

bors commented Nov 25, 2023

⌛ Testing commit ebfb95a with merge 16087ee...

@bors
Copy link
Collaborator

bors commented Nov 25, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 16087ee to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2023
@bors bors merged commit 16087ee into rust-lang:master Nov 25, 2023
@rustbot rustbot added this to the 1.76.0 milestone Nov 25, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (16087ee): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 4.7% [4.7%, 4.7%] 1
Regressions ❌
(secondary)		 2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)		 -3.9% [-3.9%, -3.9%] 1
Improvements ✅
(secondary)		 -3.7% [-4.8%, -2.2%] 4
All ❌✅ (primary) 0.4% [-3.9%, 4.7%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.133s -> 674.958s (-0.03%)
Artifact size: 313.36 MiB -> 313.33 MiB (-0.01%)

@RalfJung RalfJung deleted the unadjusted-abi branch November 27, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@davidtwco davidtwco

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.76.0
Development

Successfully merging this pull request may close these issues.

Intrinsic has incorrect return type. LLVM ERROR: Broken function found, compilation aborted. aarch64 / ARM / nightly
8 participants
@RalfJung @rustbot @Noratrieb @bjorn3 @compiler-errors @bors @rust-timer @davidtwco